-
Notifications
You must be signed in to change notification settings - Fork 117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Radio - Replace inventory PFH with CBA loadout handler #1015
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: PabstMirror <[email protected]>
maybe make a note in |
a5283b0
to
7f1b494
Compare
Does this interfere with Basic Mission Setup module? This particular functionality of it is broken atm (ref. #574), but we should still think about it. |
So one of the complexities of moving away from PFH is This exists because when right clicking radios on the ground (also crate/other person's inventory etc) they would typically take the assigned item slot. This is less of an issue now that radios inherit from a misc item so they can't go there except So a test case for this is ensuring |
Changed API function to wrapper for sys_core
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this work in combination with Basic Setup module? I think default radios in the module are kind of broken currently anyways.
Co-authored-by: jonpas <[email protected]>
This question remains and should be looked at. It kind of partly replaces that part of the module as far as I understand. @Sniperhid can you look at recent changes regarding replacement of PFH with EH? |
Haven't tried it in game yet
Just as a heads up I haven't tested the last few commits in game |
@@ -8,6 +8,7 @@ if (!hasInterface) exitWith {}; | |||
|
|||
["ace_arsenal_displayClosed", { | |||
EGVAR(sys_core,arsenalOpen) = false; | |||
[] call DFUNC(monitorRadiosHandler); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving a note here: We have to think about API for this, copy-pasting those lines is far from ideal.
Will probably take a look at finishing this for 2.10.0. |
This is almost done, but I don't want to risk it release in v2.11.0, will be part of dev-build after though. |
Split that off into #1283 so we can merge the important part and leave below for further testing.
As noted in a comment above, problem is we are potentially not catching all cases of radio replacement with the handler. If we need a special call to handle it for Arsenal, then who knows how many 3rd party systems would need to do the same - that's not reliable. System needs to catch it all without a new API requiring this. PFH as it is shouldn't be expensive in most cases anyways. |
When merged this pull request will:
Add setting for setting the replacement radioRadio - Add radio item replacement CBA Setting #1283Add option for simply removing the radio instead without replacementRadio - Add radio item replacement CBA Setting #1283Tested in vanilla arsenal